Skip to content

Add NUOPC cap and rearrange driver directory#376

Merged
apcraig merged 12 commits into
CICE-Consortium:masterfrom
apcraig:escomp/ciceA
Nov 2, 2019
Merged

Add NUOPC cap and rearrange driver directory#376
apcraig merged 12 commits into
CICE-Consortium:masterfrom
apcraig:escomp/ciceA

Conversation

@apcraig
Copy link
Copy Markdown
Contributor

@apcraig apcraig commented Oct 29, 2019

PR checklist

  • Short (1 sentence) summary of your PR:
    Add NUOPC cap and rearrange the driver directory
  • Developer(s):
    apcraig
  • Suggest PR reviewers from list in the column to the right.
  • Please copy the PR test results link or provide a summary of testing completed below.
    Tested on cheyenne, full test suite, bit-for-bit
    Also tested on a CESM2 T62_g37 DTEST compset, successfully ran 5 days with CMEPS
  • How much do the PR code changes differ from the unmodified code?
    • bit for bit
    • different at roundoff level
    • more substantial
  • Does this PR create or have dependencies on Icepack or any other models?
    • Yes
    • No
  • Does this PR add any new test cases?
    • Yes
    • No
  • Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/. A test build of the technical docs will be performed as part of the PR testing.)
    • Yes
    • No, does the documentation need to be updated at a later time?
      • Yes
      • No
  • Please provide any additional information or relevant details below:

Adds driver/nuopc/cmeps which was successfully tested with CESM2 at T62_g37 with DTEST.
Renames driver/cesm to driver/mct/cesm1
Renames driver/cice to driver/standalone/cice
Renames driver/hadgem3 to driver/subroutine/hadgem3

Refactored some aspects of the nu_diag initialization because CMEPS + NUOPC introduces the requirement that the coupling cap initializes the CICE log file. I have provided feedback that this is not an ideal approach because it will create differences with other caps and other applications, but that largely went in one ear and out the other. There are other ways this could be handled in CICE6, but in the end, we want to keep the initialization in the cice_init routine, so we have to be able to "detect" when nu_diag has been set earlier in those cases.

Documentation was added to provide some information about the drivers directory.

@apcraig
Copy link
Copy Markdown
Contributor Author

apcraig commented Oct 29, 2019

I ran full test suite on gordon and izumi. Results are here, #7358329830af, https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks

Copy link
Copy Markdown
Member

@phil-blain phil-blain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have experience with pio or cesm coupling, so I won't comment on theses changes or the nu_diag stuff.
I read the updates to the documentation and I think it's good to go.

Comment thread doc/source/user_guide/ug_running.rst Outdated

- setup of the batch scripts (in **cice.batch.csh**)

- setup of the model launch (in **cice.launch.csh**)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for that! I had noticed a few months ago that we did not mention cice.launch.csh and it was on my backlog to do a PR for that before the tutorial!

Adding a New Driver
------------------------

The drivers directory contains two-levels of subdirectories. The first layer indicates the coupling infrastructure or strategy and the second later indicates the application or coupler the driver is written for. At the present time, the directory structures looks like::
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"two-levels" -> "two levels"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, this is clearly incorrect. I have just pushed a fix.

@phil-blain
Copy link
Copy Markdown
Member

side note: the Travis CI build succeeded but its status is not updated in the PR page, I wonder why...

Copy link
Copy Markdown
Contributor

@eclare108213 eclare108213 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a couple of very minor editorial changes:
In dg_driver.rst, change

At the present time, the directory structures looks like::

to

At the present time, the directory structure is

In ug_running.rst, change "four basis issues" to "four basic issues"

@apcraig
Copy link
Copy Markdown
Contributor Author

apcraig commented Oct 29, 2019

Thanks @eclare108213, I have updated the documentation again. Will wait for checks to complete then will probably merge.

@eclare108213
Copy link
Copy Markdown
Contributor

Aren't all of the driver interfaces essentially calling subroutines?

@apcraig
Copy link
Copy Markdown
Contributor Author

apcraig commented Oct 29, 2019

@eclare108213, that's largely true unless you are running multiple executable and coupling thru a low level interface. The difference between subroutine and mct/nuopc/esmf/oasis is the subroutine interfaces are relatively arbitrary and defined by the application, not by a formal coupling interface. Think of an ocean model calling cice from the middle of it's subroutines with some arbitrary calls, maybe directly into cice or maybe into some layer on cice. It's not really a formal layer of infrastructure.

I thought about whether subroutine was a good directory name and decided it was ok. In hindsight, maybe it isn't. We can rename the "subroutine" driver directory if you'd like. How about "subprogram" or "other" or something else? Anyone have any other ideas?

@eclare108213
Copy link
Copy Markdown
Contributor

I was thinking the same thing. The main idea is that this form of coupling treats the ice model much less independently, more integrated. What about something like 'subsidiary' or 'direct' or something like that?

@apcraig
Copy link
Copy Markdown
Contributor Author

apcraig commented Oct 30, 2019

I like direct over subroutines and have just changed that. Good idea.

@apcraig
Copy link
Copy Markdown
Contributor Author

apcraig commented Nov 1, 2019

Just FYI that I am going to merge this today unless there are any other comments. I'd like to get it in before the automated testing for the weekend if it's ready.

@apcraig apcraig merged commit 85f7dac into CICE-Consortium:master Nov 2, 2019
@apcraig apcraig mentioned this pull request Dec 9, 2019
setenv ICE_HSTDIR ${ICE_RUNDIR}/history
setenv ICE_LOGDIR ${ICE_CASEDIR}/logs
setenv ICE_DRVOPT cice
setenv ICE_DRVOPT standalone/cice
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@apcraig I noticed that this change from drivers/cice to drivers/standalone/cice was not reflected in the cice.settings documentation, that still references cice. Minor, but still thought I'd mention it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I'll update it in a sandbox that I plan to PR soon. Thanks!

@apcraig apcraig deleted the escomp/ciceA branch August 17, 2022 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants